Skip to content

test: replace HMR navigation helper with fixture readiness - #23853

Open
leracherry wants to merge 1 commit into
vitejs:mainfrom
leracherry:fix/hmr-optional-chaining-timeout
Open

leracherry wants to merge 1 commit into
vitejs:mainfrom
leracherry:fix/hmr-optional-chaining-timeout

Conversation

@leracherry

@leracherry leracherry commented Oct 20, 2026 •

Copy link
Copy Markdown

After an HMR update, navigation can briefly load the bundled-dev fallback page. A test that edits a file before the final page reconnects can lose the update.

Remove gotoLatestBuild and wait for a DOM marker set by the fixture after an HMR round trip. This lets the test wait for the page it will actually edit. The file-restoration test also waits for the parent update before the next navigation.

Refs #23028.

Rebased onto db8b119. The build and all three HMR specs passed locally: 44 tests in bundled dev and 63 in normal dev. The delayed-registration regression timed out without the readiness wait in the earlier validation. The previous CI run passed on Linux, macOS, and Windows.

@leracherry leracherry changed the title test: wait for bundled HMR clients after navigation test: wait for HMR readiness before navigating Oct 20, 2026
@leracherry
leracherry force-pushed the fix/hmr-optional-chaining-timeout branch from bd4511e to 6b8c968 Compare October 20, 2026 23:35
@h-a-n-a

h-a-n-a commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Hmmm, this may not be the correct way to fix it. We should probably remove gotoLatestBuild completely. But thanks for the contribution though.

@leracherry

Copy link
Copy Markdown
Author

Thanks for reviewing. I tested replacing all three gotoLatestBuild call sites with page.goto, keeping the regression test. That produced four bundled-dev failures: reconnect logs reached later assertions, and one readiness check finished before used:foo0 arrived.

Would you prefer replacing the helper with browser-observable readiness/reload checks, or addressing the navigation/registration behavior in production first? I restored the experiment so the current patch is unchanged while I clarify the direction.

@leracherry
leracherry marked this pull request as draft October 21, 2026 16:49
@leracherry

leracherry commented Oct 26, 2026 •

Copy link
Copy Markdown
Author

Reworked this to remove gotoLatestBuild entirely. The navigations now wait for a DOM marker set after a fixture-owned HMR round trip. This handles fallback reloads and registration without inspecting private server state.

All three HMR specs pass in bundled and normal dev. The delayed-registration regression fails with the readiness wait removed. The description has the current validation results. Could you review this revised approach?

@leracherry
leracherry marked this pull request as ready for review October 26, 2026 00:37
@leracherry leracherry changed the title test: wait for HMR readiness before navigating test: replace HMR navigation helper with fixture readiness Oct 26, 2026
@leracherry
leracherry force-pushed the fix/hmr-optional-chaining-timeout branch from 09c5427 to edc5b11 Compare November 9, 2026 19:56
@leracherry

Copy link
Copy Markdown
Author

Following up on the revised approach: gotoLatestBuild has been removed and replaced with fixture-owned readiness checks, as described above. The latest CI run passes and the branch is conflict-free. Could you take another look when convenient?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants